contractcourt: use the sweeper for HTLC offered remote timeout resolu… #9062
Conversation
Roasbeef
added
enhancement
|
Important Review skippedAuto reviews are limited to specific labels. Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Roasbeef
force-pushed
the
htlc-resolution-sweeper
branch
from
fb5c245 to
3da8e85
Compare
There was a problem hiding this comment.
Yeah I also noticed this in #8922 where the direct spend is not offered to the sweeper and was planning to fix it afterwards. I think we need to handle the case where there's already a force close in the process and the output has already been offered to the utxo nursery. In that case, during the startup, we need to delete it from the nursery and offer it to the sweeper?
My idea to handle that is if the close is already in process, and offered, then this proceeds as normal (re the |
Roasbeef
force-pushed
the
htlc-resolution-sweeper
branch
from
3da8e85 to
fba0059
Compare
|
Pushed up a new version, itests+unit tests should be passing now. It also starts to send all new remote HTLC timeout outputs to the sweeper (prior commit was just taproot outputs). |
With the latest version, I modified the logic slightly:
I also needed to make a change to the way we generate reports for outputs. Previously it would skip asking the |
Roasbeef
force-pushed
the
htlc-resolution-sweeper
branch
2 times, most recently
from
6c191bd to
2491692
Compare
There was a problem hiding this comment.
Is there a chance we fix this after #8922 - think things would be more clear there. However if the custom channel PR depends on this then nvm.
Originally I set out to fix some existing TODOs/bugs as relates to the new aux chan feature, then realized that I'd have to update the nursery to understand some of the new interfaces. Instead of doing that, I opted to do this existing item on our wish list so things are more uniform. With this PR, this issue (assuming all the edge cases re upgrades are accounted for) is in sight: #3688 |
Along the way we refactor the test to eliminate some unnecessary line length.
…tion In this commit, we bring the timeout resolver more in line with the success resolver by using the sweeper to handle the HTLC offered remote timeout outputs. These are outputs that we can sweep directly from the remote party's commitment transaction when they broadcast their version of the commitment transaction. With this change, we slim down the scope slightly by only doing this for anchor channels. Non-anchor channels will continue to use the utxonursery for this output type for now.
Roasbeef
force-pushed
the
htlc-resolution-sweeper
branch
from
2491692 to
903c8fc
Compare
…tion
In this commit, we bring the timeout resolver more in line with the success resolver by using the sweeper to handle the HTLC offered remote timeout outputs. These are outputs that we can sweep directly from the remote party's commitment transaction when they broadcast their version of the commitment transaction.
With this change, we slim down the scope slightly by only doing this for anchor channels. Non-anchor channels will continue to use the utxonursery for this output type for now.